Fix clang 11 and make icudata embed portable - #3
Merged
Merged
Conversation
This is more broadly available, e.g. it's the default on Debian Bullseye. This also means we don't need to hard-code the full path to Clang anymore, which is pretty nice.
jasnell
reviewed
Sep 19, 2022
Collaborator
There was a problem hiding this comment.
Should we explicitly recommend at least clang 15 still?
Member
Author
There was a problem hiding this comment.
Nah, I don't think it matters. The only difference I'm aware of is the one missing use of concepts but it doesn't seem like enough to tell people to do extra work.
jasnell
approved these changes
Sep 19, 2022
wd_cc_embed doesn't seem to work on Mac and it was magic copy-pasta to start with so I'm not sure how to fix it. This approach is portable, but does result in a 57MB generated C++ file which is kinda gross.
kentonv
force-pushed
the
kenton/clang-11
branch
from
September 19, 2022 19:21
48c862d to
bbff5bf
Compare
mrbbot
approved these changes
Sep 19, 2022
mrbbot
added a commit
to mrbbot/workerd
that referenced
this pull request
Jun 22, 2023
This was referenced Mar 5, 2026
10 tasks
netanelgilad
added a commit
to netanelgilad/workerd
that referenced
this pull request
Jul 2, 2026
…wn (fork gap cloudflare#3) Fork gap cloudflare#3 reported that a nested native child_process.spawn() from a NON-drainProcess child hit EPERM on VFS writes (a registry fetch's cacache mkdir failed), while the same write from a drain child succeeded -- the drain/non-drain asymmetry was the sharp edge iso worked around by making every generic child a drain child. Investigation shows the write-permission path has NO drain-conditional gate on this branch: RootDirectory::add (io/worker-fs.c++) returns NOT_PERMITTED only when no shared writable store is reachable (tryGetSharedStore -> IoContext / module-eval fallback / stack scope), which is independent of drainProcess. The gate that produced gap cloudflare#3 was the PRE-re-root read-only root: before 'spike(vfs-root-mount): re-root the shared writable VFS at / instead of /tmp', only /tmp was writable and '/' was read-only, so cacache/npm writes to cache paths OUTSIDE /tmp hit the read-only root -> NOT_PERMITTED (surfacing as EPERM). The re-root routes the ENTIRE root to the shared writable store, so the fix is already present; no runtime change is required. This regression test pins the invariant: a nested spawn writes a cacache-shape tree (mkdir + writeFileSync under '/', OUTSIDE /tmp) and must succeed for BOTH a non-drain AND a drain spawner -- proving write permission does not depend on drain mode. It would have failed pre-re-root (write outside /tmp -> NOT_PERMITTED). Test only, no runtime behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.